Skip to content

Conversation

@ELVIS-KATO
Copy link
Contributor

@ELVIS-KATO ELVIS-KATO commented Nov 13, 2025

Requirements

  • This PR has a title that briefly describes the work done including a conventional commit type prefix and a Jira ticket number if applicable. See existing PR titles for inspiration.
  • My work is based on designs, which are linked or shown either in the Jira ticket or the description below.
  • My work includes tests or is validated by existing tests.

Summary

Modified the code so that bills can only be modified only if the status is pending. The code checks for the status before allowing someone to edit a bill.
The Edit button has been made active only when the status is pending.
I have tried to Preserve POSTED/ADJUSTED from backend so UI can block it.

Screenshots

the NEW VIDEO

Pending2.mp4

Related Issue

https://openmrs.atlassian.net/issues?filter=-4&selectedIssue=O3-5200

Other

@ibacher @denniskigen

@ibacher
Copy link
Member

ibacher commented Nov 13, 2025

@ELVIS-KATO The point of this ticket was that this needs to be fixed on both the backend and frontend. We should fix the backend first because the frontend is also going to need to respond to the errors produced by the backend. Also, you asked a good clarifying question on the ticket, but it's unaddressed here?

If the ticket needs clarifications, we need to sort that out before committing code because it probably points to things that aren't well-specified.

@ELVIS-KATO
Copy link
Contributor Author

Ok @ibacher, thanks for that. So, it means this will be valid after the backend has been refactored.

@ELVIS-KATO
Copy link
Contributor Author

I have made a second commit that is working on what @NethmiRodrigo had talked about

@NethmiRodrigo NethmiRodrigo changed the title (fix)O3-5200 Add validation so that once a bill has been "posted" its line items cannot be modified (fix) O3-5200 Add validation so that once a bill has been "posted" its line items cannot be modified Nov 26, 2025
Copy link
Contributor

@NethmiRodrigo NethmiRodrigo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ELVIS-KATO

Comment on lines 100 to 120
{canEditBill ? (
<Button
data-testid={`edit-button-${item.uuid}`}
renderIcon={Edit}
hasIconOnly
kind="ghost"
iconDescription={t('editThisBillItem', 'Edit this bill item')}
tooltipPosition="left"
onClick={() => handleSelectBillItem(item)}
/>
) : (
<Button
data-tested={`edit-button-${item.uuid}`}
renderIcon={Edit}
hasIconOnly
kind="ghost"
disabled
iconDescription={t('cannotEditThisBill', 'You can not edit this bill')}
tooltipPosition="left"
/>
)}
Copy link
Contributor

@NethmiRodrigo NethmiRodrigo Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don’t need to conditionally render a ghost button. Just disabling the button is sufficient, like so:

Suggested change
{canEditBill ? (
<Button
data-testid={`edit-button-${item.uuid}`}
renderIcon={Edit}
hasIconOnly
kind="ghost"
iconDescription={t('editThisBillItem', 'Edit this bill item')}
tooltipPosition="left"
onClick={() => handleSelectBillItem(item)}
/>
) : (
<Button
data-tested={`edit-button-${item.uuid}`}
renderIcon={Edit}
hasIconOnly
kind="ghost"
disabled
iconDescription={t('cannotEditThisBill', 'You can not edit this bill')}
tooltipPosition="left"
/>
)}
<Button
data-testid={`edit-button-${item.uuid}`}
renderIcon={Edit}
hasIconOnly
kind=“ghost”
iconDescription={t('editThisBillItem', 'Edit this bill item’)}
tooltipPosition=“left”
onClick={() => handleSelectBillItem(item)}
disabled={bill?.status !== ‘PENDING’}
/>

Comment on lines 24 to 28
const isSpecialStatus = bill.status === 'POSTED' || bill.status === 'PAID' || bill.status === 'ADJUSTED';

const status =
activeLineItems.length > 0
const status = isSpecialStatus
? bill.status
: activeLineItems.length > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t understand why we need to do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we need it coz it ensures that if the backend has already marked a bill as a final state (POSTED, PAID, or ADJUSTED), the UI keeps that status instead of recalculating one from the line items. Coz to my observation previously, the UI always derived the status from line-item states, which overwrote important server-driven statuses and incorrectly made finalized bills appear editable. The isSpecialStatus check prevents that by preserving the backend’s status so the UI can correctly block edits unless the bill is truly PENDING. But maybe I miss understood the code functionality or it could be catered for in the backend.

Comment on lines 91 to 98
if (bill?.status === 'PENDING') {
showSnackbar({
title: t('cannotEditThisBill', 'You can not edit this bill'),
subtitle: t('onlyPendingBillsCanBeEdited', 'Only pending bills can be edited'),
kind: 'error',
});
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think this is the proper place to trigger a snackbar. I don’t think we need to add a safeguard in this modal at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok let me have it removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants